-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-143120: pixi builds for free-threading and TSAN #142872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste of asan/pixi.toml
| script: | ||
| file: ../build.sh | ||
| env: | ||
| PYTHON_VARIANT: "free-threading" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste of asan/recipe.yaml, except this one line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste of asan/pixi.toml
Tools/pixi-packages/tsan/recipe.yaml
Outdated
| script: | ||
| file: ../build.sh | ||
| env: | ||
| PYTHON_VARIANT: "tsan" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste of asan/recipe.yaml, except this one line
no clue sorry I haven't seen this before |
|
The tsan crash is due to the default $ sudo sysctl vm.mmap_rnd_bits
vm.mmap_rnd_bits = 32 # too high
$ sudo sysctl vm.mmap_rnd_bits=28 # reduce it
vm.mmap_rnd_bits = 28
|
|
There's a lot of copy/paste here, which can make maintenance harder. Does pixi have some concept of code reuse or parametrisation? Maybe not the best example, but something like GitHub Actions' reusable workflows? |
|
@lucascolley is there anything we can do right now to reduce the duplication here? Or does that require fixes in pixi? |
|
As mentioned in the README already, this is blocked on prefix-dev/pixi#4599. |
ngoldbaum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened an issue which is needed to make this mergeable. I'm also playing with a NumPy recipe based on this.
Tools/pixi-packages/README.md
Outdated
| - `asan`: ASan-instrumented build with `PYTHON_ASAN=1` | ||
| - `free-threading` | ||
| - `asan`: ASan-instrumented build | ||
| - `tsan`: free-threading, TSan-instrumented build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be called tsan-free-threading. There should probably also be an asan-free-threading. @lucascolley is there a way to avoid the combinatorial explosion of variants here? Ideally you'd be able to somehow combine these but I think that's probably not tenable at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the answer to this is no because of prefix-dev/pixi#4599
Barring the above, we could write a template + generation script and then commit the output to git? Very ugly if you ask me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would vote for keeping things simple but verbose at the minute until there is a proper solution upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to tsan-free-threading
doh should have checked the CI log earlier, it is working! Just worked locally for me too |
Oh hey, it worked! That's so strange - I wonder what's different on my setup. It looks like pixi is up-to-date... |
|
if in doubt try |
Nope, doesn't work. Here's the build log after The only difference I see compared with the working build on CI is that my build used uv 0.9.22 but the CI build used 0.9.21. |
This ended up coming down to there being a |
- Remove clang-19 since we are not building the experimental jit - Remove ld_impl because we are not hacking the makefiles - Add site_packages_path - Make recipe.yaml identical with just the variant name difference - Add a generate.sh to generate recipe.yaml from default variant - Add a runtime requirement of libsanitizer for gcc
|
looks great, prefix-dev/pixi-build-backends#532 should get us down to a single |
|
Are symbolic links allowed? (Especially since these files end up in the python source distribution, it might be bad because of windows). If not we might have to duplicate the files. |
lucascolley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we update the README to point to the new blockers for using a single manifest and recipe per #142872 (comment) ?
|
Tested all builds on pixi 0.62.2 Linux x64 against ngoldbaum/numpy#4 @isuruf at the end of the numpy build I get
but I can't figure out if it's actually a problem or what it is about; could you investigate? |
| --oldincludedir="${BUILD_PREFIX}/${HOST}/sysroot/usr/include" \ | ||
| --enable-shared \ | ||
| --srcdir="${SRC_DIR}" \ | ||
| --with-system-expat \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@isuruf not sure what this is about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we install expat from conda, this tells cpython to use that.
You can ignore it. Harmless warning. |
Follow-up to #142469
freethreading: compiles with--disable-giltsan-freethreading: compiles with--disable-gil --with-thread-sanitizerrecipe.yamlandpixi.tomlall identical symlinks